Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fscrypt: create a new blank key sized according to the passphrase #4464

Merged
merged 2 commits into from
Mar 6, 2024

Conversation

mgfritch
Copy link
Contributor

@mgfritch mgfritch commented Feb 27, 2024

Padding a passphrase with null chars to arrive at a 32-byte length
later forces a user to also pass null chars via the term when
attempting to manually unlock a subvolume via the fscrypt cli tools.

This also had a side-effect of truncating any longer length passphrase
down to a shorter 32-byte length.

fixup for:
cfea8d7
dd0e198

Is there anything that requires special attention

Backward compatibility is provided to unlock subvolumes that might have been created using the old style null-padded passphrase

This PR also includes an additional fix for a related issue where fscrypt was allowed to indefinitely retry the keyFn after an auth failure, which can block subsequent requests to the ceph csi driver.

Related issues

n/a

Future concerns

List items that are not part of the PR and do not impact it's
functionality, but are work items that can be taken up subsequently.

Checklist:

  • Commit Message Formatting: Commit titles and messages follow
    guidelines in the developer
    guide
    .
  • Reviewed the developer guide on Submitting a Pull
    Request
  • Pending release
    notes

    updated with breaking and/or notable changes for the next major release.
  • Documentation has been updated, if necessary.
  • Unit tests have been added, if necessary.
  • Integration tests have been added, if necessary.

Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@mgfritch mgfritch force-pushed the fscrypt-devel branch 4 times, most recently from aa4121b to 186306a Compare February 27, 2024 06:41
Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@nixpanic nixpanic added component/cephfs Issues related to CephFS component/rbd Issues related to RBD component/util Utility functions shared between CephFS and RBD labels Feb 27, 2024
@nixpanic nixpanic requested a review from a team February 27, 2024 08:52
@nixpanic
Copy link
Member

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Feb 27, 2024

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

@Mergifyio queue

Copy link
Contributor

mergify bot commented Feb 27, 2024

queue

🛑 The pull request has been removed from the queue default

The queue conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Feb 27, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Feb 27, 2024
@nixpanic
Copy link
Member

Looks like pulling images in the CI failed, seems to be common on Tuesdays for some weird reason.

@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Feb 27, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Feb 27, 2024
@nixpanic
Copy link
Member

nixpanic commented Mar 6, 2024

@Mergifyio rebase

fscrypt will infinitely retry the keyFn during an auth failure,
preventing the csi driver from progressing when configured with
an invalid passphrase

See also:
https://github.com/google/fscrypt/blob/8c12cd64ab471d0a73ef4c300d7c40077cad5d5d/actions/callback.go#L102-L106

Signed-off-by: Michael Fritch <mfritch@suse.com>
Padding a passphrase with null chars to arrive at a 32-byte length
later forces a user to also pass null chars via the term when
attempting to manually unlock a subvolume via the fscrypt cli tools.

This also had a side-effect of truncating any longer length passphrase
down to a shorter 32-byte length.

fixup for:
cfea8d7
dd0e198

Signed-off-by: Michael Fritch <mfritch@suse.com>
Copy link
Contributor

mergify bot commented Mar 6, 2024

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

nixpanic commented Mar 6, 2024

@Mergifyio queue

@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Mar 6, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.28

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.27

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Mar 6, 2024
@mergify mergify bot merged commit 3410687 into ceph:devel Mar 6, 2024
35 checks passed
@mgfritch mgfritch deleted the fscrypt-devel branch March 6, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/cephfs Issues related to CephFS component/rbd Issues related to RBD component/util Utility functions shared between CephFS and RBD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants